docs(memos): sync template standards updates#349
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughUpdates the memos playground config with ingress and network policy options, expands the memos chart documentation to describe those settings, and registers memos in the playground site-sync mapping. ChangesMemos playground and docs
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/data/playground-configs.ts (1)
8140-8146: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winMemos ingress class options lack the empty option now documented in memos.mdx.
The updated
memos.mdxdocuments settingingress.ingressClassName: ""to omitspec.ingressClassName, mirroring the pattern already used forapacheand nowolivetin(options: ['', ...]). Thememosentry here still only offers['traefik', 'nginx'], so playground users can't exercise the documented behavior.♻️ Proposed fix
{ label: 'Ingress Class', key: 'ingress.ingressClassName', type: 'select', default: 'traefik', - options: ['traefik', 'nginx'], + options: ['', 'traefik', 'nginx'], description: 'Ingress controller class', },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/data/playground-configs.ts` around lines 8140 - 8146, The Memos ingress class selector is missing the empty option that is now documented, so update the playground config entry for ingress.ingressClassName to include an empty choice alongside the existing traefik and nginx values. Use the existing pattern from the apache and olivetin config entries in playground-configs.ts, keeping the same label, key, and description while extending the options list so users can select "" and omit spec.ingressClassName.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/data/playground-configs.ts`:
- Around line 8140-8146: The Memos ingress class selector is missing the empty
option that is now documented, so update the playground config entry for
ingress.ingressClassName to include an empty choice alongside the existing
traefik and nginx values. Use the existing pattern from the apache and olivetin
config entries in playground-configs.ts, keeping the same label, key, and
description while extending the options list so users can select "" and omit
spec.ingressClassName.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7368d274-6320-4af0-abbf-bec0881de11f
📒 Files selected for processing (3)
src/data/playground-configs.tssrc/pages/docs/charts/memos.mdxsrc/pages/playground.astro
|
Synced the Memos NetworkPolicy docs with helmforgedev/charts#671. What changed:
Validation:
|
|
Addressed the CodeRabbit review-body feedback. Changes:
Validation:
Note: this CodeRabbit item was posted in the review summary/body, not as an active review thread, so there is no thread ID to reply to or resolve. |
|
Synced the Memos site docs with the chart update in helmforgedev/charts#671. Changes:
Validation:pm run format:check passed.pm run lint passed.pm run build passed.
|
## Summary - align memos template standards: immutable selector labels, guarded ingressClassName, centralized validate helper, and numbered NOTES - add networkPolicy.extraEgress with built-in DNS and HTTPS baseline egress when egress isolation is enabled - keep Memos-specific StatefulSet/database validation behavior while adding selector-label override coverage ## Validation - helm unittest charts/memos: 20 tests passed - helm lint --strict charts/memos - make template-standards-check CHART=memos - make standards-check CHART=memos - make standards-guard CHART=memos - node scripts/charts/validate-chart.js --chart memos --no-k3d: all local layers passed except local Windows remote kubeconform schema download from raw.githubusercontent.com - strict kubeconform rerun with local official schemas: default plus all ci/*.yaml passed, Skipped: 0 - k3d behavioral validation on k3d-helmforge-tests-wsl: default plus all ci/*.yaml passed Issue: #633 Site PR: helmforgedev/site#349 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added NetworkPolicy egress isolation with built-in DNS/HTTPS allowances and support for appending extra egress rules. * Improved installation notes with clearer access, workload, configuration, persistence/database details, exposure status, and troubleshooting. * **Bug Fixes** * Omit `ingressClassName` in rendered Ingress when left empty. * Align StatefulSet pod labels with the selector set. * Strengthened chart validation for invalid replica/database/persistence and restricted selector label overrides. * **Documentation** * Expanded Memos chart README networking defaults and egress behavior guidance. * **Tests** * Added/updated tests for NetworkPolicy egress rendering, baseline customization, rule ordering, ingress omission, and validation failures. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: MergeCheck <mergecheck@example.invalid>
8852b63 to
3611ee2
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/data/playground-configs.ts (1)
8424-8451: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider exposing DNS/extra-egress controls to match documented behavior.
Per the PR comments, the docs describe
networkPolicy.dnsEgress(with itskube-system/k8s-app: kube-dnsdefault selector) andnetworkPolicy.extraEgress(append custom rules while isolation is enabled) alongsideegressIsolation. This section only exposesegressIsolationand the HTTPS CIDR fields, unlike thelangflowandkubernetes-mcp-serverconfigs in this same file, which expose analogousdnsEgressPeers[...]andextraEgress[...]fields for their NetworkPolicy sections. If parity with the documentation is desired in the playground, consider adding fields for these two keys.♻️ Optional fields to add for parity with docs
{ name: 'Network Policy', collapsible: true, gateField: 'networkPolicy.enabled', fields: [ { label: 'Egress Isolation', key: 'networkPolicy.egressIsolation', type: 'toggle', default: 'false', description: 'Render baseline DNS and HTTPS egress rules', }, { label: 'HTTPS IPv4 CIDR', key: 'networkPolicy.httpsEgress[0].ipBlock.cidr', type: 'text', default: '0.0.0.0/0', description: 'IPv4 HTTPS egress destination', }, { label: 'HTTPS IPv6 CIDR', key: 'networkPolicy.httpsEgress[1].ipBlock.cidr', type: 'text', default: '::/0', description: 'IPv6 HTTPS egress destination', }, + { + label: 'Extra Egress CIDR', + key: 'networkPolicy.extraEgress[0].to[0].ipBlock.cidr', + type: 'text', + default: '', + description: 'Optional additional egress destination', + }, ], },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/data/playground-configs.ts` around lines 8424 - 8451, The Network Policy playground section exposes only egressIsolation and HTTPS CIDR fields, so it is missing the documented dnsEgress and extraEgress controls. Update the Network Policy block in playground-configs by adding fields for networkPolicy.dnsEgress and networkPolicy.extraEgress, following the same pattern used by the langflow and kubernetes-mcp-server NetworkPolicy sections. Make sure the DNS field reflects the kube-system / k8s-app: kube-dns default selector and the extra-egress field lets users append custom rules while isolation is enabled.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/data/playground-configs.ts`:
- Around line 8424-8451: The Network Policy playground section exposes only
egressIsolation and HTTPS CIDR fields, so it is missing the documented dnsEgress
and extraEgress controls. Update the Network Policy block in playground-configs
by adding fields for networkPolicy.dnsEgress and networkPolicy.extraEgress,
following the same pattern used by the langflow and kubernetes-mcp-server
NetworkPolicy sections. Make sure the DNS field reflects the kube-system /
k8s-app: kube-dns default selector and the extra-egress field lets users append
custom rules while isolation is enabled.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 0878ba23-c53d-443b-9895-ea888d190f68
📒 Files selected for processing (3)
src/data/playground-configs.tssrc/pages/docs/charts/memos.mdxsrc/pages/playground.astro
✅ Files skipped from review due to trivial changes (1)
- src/pages/docs/charts/memos.mdx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/pages/playground.astro
Summary
Validation
Chart PR: helmforgedev/charts#671
Issue: helmforgedev/charts#633
Summary by CodeRabbit
memoschart playground with an expanded “Ingress Class” selector that includes an empty option.memosplayground (shown when enabled), including “Egress Isolation” and HTTPS IPv4/IPv6 CIDR fields (defaults: disabled,0.0.0.0/0,::/0).memosin site-synced playground chart filtering.memosconfiguration reference for Ingress and NetworkPolicy, including behavior when Ingress Class is empty and how egress isolation affects outbound DNS/HTTPS.